tbf the situation with 18.20.5 feels very very uncommon - there's usually not much of a delay at all between updating the tab file and uploading the artifacts.

i'd think a better approach than an option like this is to filter out versions that don't have files present on the server, in nv itself.

Does nodejs.org currently provide an endpoint that could tell us that with a single request (e.g. extended version of dist/index.json)? Otherwise it could be a tad inefficient to determine this, depending on how many aliases/versions we're evaluating

You'd only need to check versions that were released within the past 24 hours, that were also the latest version of the release line.

That's fair, that wouldn't be so bad.

Really though doing anything within nv around this feels like a workaround for a bug in node's release process. i.e. as a user of node, I would expect that when a version appears in dist/index.json (or whatever), I should immediately be able to download it, without having to do extra calls, similar to https://registry.npmjs.org/<somepackage>. I'm sure that's easier said than done though ❤️

Indeed - tbh I'm not sure it's worth working around it with anything, including this PR :-)

closing, as the v18.20.5 dist issue is resolved 😅

if this problem resurfaces again, a workaround could be to bypass cloudflare altogether and download binaries from https://direct.nodejs.org/dist/ (assuming that's supported)

I dislike closing this. I understand the feedback and saying it is not really a problem from the perspective of today's issue, but my point stands that I think this behavior is probably more what users might expect with the --now flag. @ljharb I didn't see you address that above, do you disagree on that part?

feels like a workaround for a bug in node's release process

I am following up with this here: https://openjs-foundation.slack.com/archives/CK9Q4MB53/p1731450996448959

EDIT: I didn't re-open although I could, but I wanted to just say that so @jenseng could decide (as it is your PR), but that is personally what I would prefer we do so it does not appear "decided" when we only just had a convo today and not many other folks have had a chance to comment. Typically I would only close something this quickly if it was CLEARLY invalid or never going to happen, which I don't think we achieved consensus on in the above convo.

Sure, although my immediate need is resolved, I'm happy to leave this open for further feedback/discussion.

I also agree that in the long term the now flag should just have this behavior, as it makes the most sense (though that would be a breaking change if anyone besides me is using now 😆)

i probably didn't understand what the PR does.

certainly if an explicit "current time" timestamp is passed, releases from after it should simply not appear to exist, just like npm's --before flag.

Yep, I think my initial use for this was more for testing and I coupled it with input which worked with that design. But in hind sight I think this new approach is a much better api, especially for end users (not just testing).

I feel like, despite this now being open for too long, that we ended on a resolution that we like this and would rather just make this the default. With that in mind, @jenseng do you want to pick that up or would you rather I just knock it out? I am planning on picking up the #34 in the next month or so, so would be a good time to land this if we want.

Pushed a warning message for this. Once we land this on the 0.x line I will open a PR changing the defaults for the 1.x line. I think we might want to keep the option around though so we can negate it, want to think through that before committing to deprecating the option.

I'm still unclear on what this PR does or why it's needed.

There was a race condition between the data and the actual artifacts being available on the website. In that time we used it to build our internal versions and that failed in a mysterious looking way. Basically this is a bug fix to cover over the fact the Node.js build goes out in a non-atomic way.

And ignoring that (hopefully) one-off issue, this PR makes the existing now behavior sensible, which is important for testing. Without this PR, now is only used to determine whether a major is supported / active / lts / lts_active, not whether a particular version exists. So a lot of the tests are brittle and subject to break if a new patch/minor release were to land in those tested majors. I.e. many current test assertions reflect something newer than now

i.e. ignoreFutureReleases=true should really be the default behavior, but that's technically a breaking change which is why i made it an option

Ok, going to merge this for the 0.x line and then we will follow with a fix for the now behavior for 1.0.